-
Notifications
You must be signed in to change notification settings - Fork 1.9k
IGNITE-27626 Use MessageSerializer for custom messages #12651
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
f529d25 to
6ee242b
Compare
4e7c1db to
bae6713
Compare
50cdcd5 to
f87dbb3
Compare
|
| * @param caches Collection of cache names. | ||
| */ | ||
| public CacheStatisticsModeChangeMessage(UUID reqId, Collection<String> caches, boolean enabled) { | ||
| public CacheStatisticsModeChangeMessage(IgniteUuid id, UUID reqId, Collection<String> caches, boolean enabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always pass IgniteUuid.randomUuid() for the id argument in the constructor.
Maybe we should remove id from the constructor arguments and just initialize it inside the constructor?
This also applies to the private constructor.
By the way, we could also remove the UUID reqId argument and initialize it as UUID.randomUUID() instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable.
| serMsg = (Message)msg; | ||
| else { | ||
| assert msgBytes == null || msg.isMutable() : "Message bytes are not null for immutable message: bytes=" + | ||
| Arrays.toString(msgBytes) + "]"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove + “]”
| msg = (DiscoveryCustomMessage)serMsg; | ||
| else { | ||
| try { | ||
| msg = U.unmarshal(marsh, msgBytes, ldr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a check that msgBytes != null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, if msgBytes == null, it is an incorrect state of message. Unmarshal will fail with an asserion error.




No description provided.